lib/pull: Allow downgrade protection with overrides

There's a valid use case for enabling the timestamp downgrade check
while still also using override commits.

We'll make use of this in Fedora CoreOS, where the agent specifies the
exact commit to upgrade to, while still enforcing that it be newer.

Closes: #1891
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index bb7bc02b5a8c110d4edac60f4329b551735431e1..67b28f8cbd176cb4cb70e9c56847c7a1c7e915ba 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -4335,16 +4335,16 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                        const char*, override_commitid)
     {
       g_autofree char *contents = NULL;
+      g_autoptr(OstreeCollectionRef) ref_with_collection = NULL;
 
       /* Support specifying "" for an override commitid */
       if (override_commitid && *override_commitid)
         {
-          g_hash_table_replace (updated_requested_refs_to_fetch, ostree_collection_ref_dup (ref), g_strdup (override_commitid));
+          ref_with_collection = ostree_collection_ref_dup (ref);
+          contents = g_strdup (override_commitid);
         }
       else
         {
-          g_autoptr(OstreeCollectionRef) ref_with_collection = NULL;
-
           if (pull_data->summary)
             {
               gsize commit_size = 0;
@@ -4367,29 +4367,29 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
 
               ref_with_collection = ostree_collection_ref_dup (ref);
             }
+        }
 
-          /* If we have timestamp checking enabled, find the current value of
-           * the ref, and store its timestamp in the hash map, to check later.
+      /* If we have timestamp checking enabled, find the current value of
+       * the ref, and store its timestamp in the hash map, to check later.
+       */
+      if (pull_data->timestamp_check)
+        {
+          g_autofree char *from_rev = NULL;
+          if (!ostree_repo_resolve_rev (pull_data->repo, ref_with_collection->ref_name, TRUE,
+                                        &from_rev, error))
+            goto out;
+          /* Explicitly store NULL if there's no previous revision. We do
+           * this so we can assert() if we somehow didn't find a ref in the
+           * hash at all.  Note we don't copy the collection-ref, so the
+           * lifetime of this hash must be equal to `requested_refs_to_fetch`.
            */
-          if (pull_data->timestamp_check)
-            {
-              g_autofree char *from_rev = NULL;
-              if (!ostree_repo_resolve_rev (pull_data->repo, ref_with_collection->ref_name, TRUE,
-                                            &from_rev, error))
-                goto out;
-              /* Explicitly store NULL if there's no previous revision. We do
-               * this so we can assert() if we somehow didn't find a ref in the
-               * hash at all.  Note we don't copy the collection-ref, so the
-               * lifetime of this hash must be equal to `requested_refs_to_fetch`.
-               */
-              g_hash_table_insert (pull_data->ref_original_commits, ref_with_collection,
-                                   g_steal_pointer (&from_rev));
-            }
-
-          g_hash_table_replace (updated_requested_refs_to_fetch,
-                                g_steal_pointer (&ref_with_collection),
-                                g_steal_pointer (&contents));
+          g_hash_table_insert (pull_data->ref_original_commits, ref_with_collection,
+                               g_steal_pointer (&from_rev));
         }
+
+      g_hash_table_replace (updated_requested_refs_to_fetch,
+                            g_steal_pointer (&ref_with_collection),
+                            g_steal_pointer (&contents));
     }
 
   g_hash_table_unref (requested_refs_to_fetch);

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 9d51b15161c7c43631ad580ce2fa60914209fe92..a8bc49a9eee31bb450e6db14737d60f85bab2d27 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -334,6 +334,12 @@ if ${CMD_PREFIX} ostree --repo=repo pull -T origin main 2>err.txt; then
 fi
 assert_file_has_content err.txt "Upgrade.*is chronologically older"
 assert_streq ${newrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)"
+# And also check we can't pull it when using overrides
+if ${CMD_PREFIX} ostree --repo=repo pull -T origin main@${newrev2} 2>err.txt; then
+    fatal "pulled older commit override with timestamp checking enabled?"
+fi
+assert_file_has_content err.txt "Upgrade.*is chronologically older"
+assert_streq ${newrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)"
 # But we can pull it without timestamp checking
 ${CMD_PREFIX} ostree --repo=repo pull origin main
 echo "ok pull timestamp checking"